Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add direct formatting option to ImageKeyword struct with tests #63

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

add direct formatting option to ImageKeyword struct with tests #63

wants to merge 2 commits into from

Conversation

mileslucas
Copy link
Contributor

For example usage, see the code in the test files ImCreate_cube.c and ImCreate_img.c.

Key points:

  • specify F keyword type for formatted keywords
  • Format as added in the place of the space-padding uint64 in the ImageKeyword struct. The number of bytes is, by default, set to 8 to match the original padding of the struct.
    • This should be sufficient for all format strings as they are %-16.16s, which is 8 bytes, but if we need breathing room, we can save the format strings without the % since it is redundant.
  • Struct version incremented to 2.00 -> 2.01; this is not a breaking change, just a feature addition.

To fully implement these changes, code will need to be changed in milk (save_fits.c) and camstack.

@mileslucas mileslucas mentioned this pull request Jun 6, 2024
Open
@DasVinch
Copy link
Member

DasVinch commented Jun 6, 2024 •
edited
Loading

Unfortunately, I have comments...

Changing type to 'F' is breaking
--> We need to hang on to native formats everywhere, and at the very last step of FITS-ifying we need to figure out that .format is not a null. Otherwise, all the other stuff loses capability to distinguish D, F and S.

Now, does the FITS writer know and enforce that a keyword is formattable?

  • Having non null format field: yes but then everytime a keyword is reused, the format field MUST positively be cleared

  • Add new formatters 'd' 'f' 's' for formatted int, float, string: that somewhat works, and lower/upper lets the FITS writer know whether the format is current or stale, and lets camstack set the formatters to lowercase based on the knowledge that there indeed is a valid formatter. HOWEVER, every single code that performs a kw.type check against 'D', 'S', 'F' must be changed to compare (0x7f & kw.type == kw.type.upper) instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mileslucas @DasVinch